Implement IPInterfaceProvider for WindowsUserland#322
Implement IPInterfaceProvider for WindowsUserland#322Icegrave0391 wants to merge 11 commits intomainfrom
Conversation
Icegrave0391
commented
Sep 7, 2025
- Use wintun to implement IPInterfaceProvider
- Added testcases
|
It seems that |
|
@jaybosamiya-ms I fixed the CI by only including the dependency into |
jaybosamiya-ms
left a comment
There was a problem hiding this comment.
Thanks for working on this, I've added a few comments.
| /// WinTun session for networking | ||
| wintun_session: std::sync::RwLock<Option<Arc<wintun::Session>>>, |
There was a problem hiding this comment.
I am not sure I understand why this is an RwLock<Option<Arc<. Is this solely because of the exit handling?
| Ok(adapter) => adapter, | ||
| Err(_) => { | ||
| // If opening failed (adapter doesn't exist), create a new one | ||
| wintun::Adapter::create(&wintun, adapter_name, "LiteBox", None) |
There was a problem hiding this comment.
Not sure why the tunner type is LiteBox here, please include a comment
| if packet_bytes.len() != packet.len() { | ||
| unimplemented!("unexpected size {}", packet_bytes.len()) | ||
| } |
There was a problem hiding this comment.
I think this should just be an assert_eq!(..., ...); unless you have good reason to believe that the two might differ
| // Try to receive a packet from WinTun (non-blocking) | ||
| session | ||
| .try_receive() | ||
| .map_err(|_| litebox::platform::ReceiveError::WouldBlock) |
There was a problem hiding this comment.
I don't think this is correct; the errors that can happen here are not a "this would block" but instead these: https://docs.rs/wintun/latest/wintun/enum.Error.html
There was a problem hiding this comment.
I think currently we only have one WouldBlock error in litebox::platform::ReceiveError. We may want to add more typt to the platform's error
There was a problem hiding this comment.
For now, you can just throw in that case
|
|
||
| #[test] | ||
| fn test_ip_interface_without_wintun() { | ||
| // Test IP interface functionality when WinTun is not enabled | ||
| let platform = WindowsUserland::new(None); | ||
|
|
||
| // Create a sample IP packet (minimal IPv4 header) | ||
| let mut test_packet = [0u8; 64]; | ||
| test_packet[0] = 0x45; // Version 4, Header length 20 bytes | ||
| test_packet[9] = 0x06; // Protocol: TCP | ||
|
|
||
| // Test send_ip_packet without WinTun - should panic with unimplemented | ||
| let result = std::panic::catch_unwind(|| platform.send_ip_packet(&test_packet)); | ||
| assert!( | ||
| result.is_err(), | ||
| "send_ip_packet should panic when WinTun is not available" | ||
| ); | ||
|
|
||
| // Test receive_ip_packet without WinTun - should panic with unimplemented | ||
| let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { | ||
| let mut receive_buffer = [0u8; 1500]; | ||
| platform.receive_ip_packet(&mut receive_buffer) | ||
| })); | ||
| assert!( | ||
| result.is_err(), | ||
| "receive_ip_packet should panic when WinTun is not available" | ||
| ); | ||
| } |
There was a problem hiding this comment.
I don't see how this negative test is particularly helpful. In the future, if we ever did stuff without wintun, then this test would fail, but that would not be a bad thing, no? Probably can be deleted
| } | ||
|
|
||
| #[test] | ||
| #[ignore] // This test requires Administrator privileges and WinTun driver to be installed |
There was a problem hiding this comment.
Does this actually fail on CI? I'm not fully sure how it helps to add a whole bunch of ignored tests. You can see the Linux one we actually do extra things on the CI to make sure we can run the CI tests. Is that possible here?
|
🤖 SemverChecks 🤖 No breaking API changes detected Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered. |
|
It looks like |